Skip to content

test: add fee timer blocking test coverage#70

Open
xyephy wants to merge 2 commits intostratum-mining:masterfrom
xyephy:2025/11/fee-timer-test
Open

test: add fee timer blocking test coverage#70
xyephy wants to merge 2 commits intostratum-mining:masterfrom
xyephy:2025/11/fee-timer-test

Conversation

@xyephy
Copy link
Copy Markdown
Contributor

@xyephy xyephy commented Nov 27, 2025

Add test coverage for the fee-based rate limiting timer that was marked with a TODO in template_provider.cpp. The timer blocks fee-based template updates until fee_check_interval seconds have passed (-sv2interval flag).

The test uses is_test=false to exercise the actual timer logic that is normally bypassed in tests. It verifies:

  • Fee increases are blocked when the timer hasn't fired
  • Fee increases are allowed after the timer fires

Also adds a TPTester constructor overload to accept custom options, allowing tests to configure is_test and fee_check_interval independently.

Closes #67

@xyephy xyephy marked this pull request as draft November 27, 2025 18:16
TPTester tester{opts};

tester.handshake();
tester.handshake();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why the original has two handshake calls, that might be a bug.

It would be good to reduce the amount of duplicated test code, that also makes it more clear how each test case is different.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me do a clean up and have a much clearer approach

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d54d4b7 is better, but the code between coinbase_output_constraint_bytes and BOOST_REQUIRE_EQUAL(initial_bytes, expected_pair_bytes) is still duplicated. Can you extract that into one or more helpers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I have addressed it in ef1a258 . Thank you.

@xyephy xyephy force-pushed the 2025/11/fee-timer-test branch from fffb55b to d54d4b7 Compare November 28, 2025 20:22
@xyephy xyephy marked this pull request as ready for review December 9, 2025 08:11
@Sjors
Copy link
Copy Markdown
Collaborator

Sjors commented Feb 10, 2026

marked with a TODO in template_provider.cpp

Does that mean the TODO can go away?

@xyephy
Copy link
Copy Markdown
Contributor Author

xyephy commented Feb 11, 2026

marked with a TODO in template_provider.cpp

Does that mean the TODO can go away?

yes

@xyephy xyephy force-pushed the 2025/11/fee-timer-test branch from d54d4b7 to ef1a258 Compare February 11, 2026 20:56
}

BOOST_REQUIRE_MESSAGE(seen_prev_hash, std::string("Missing SetNewPrevHash during ") + context);
BOOST_REQUIRE_MESSAGE(seen_new_template, std::string("Missing NewTemplate during ") + context);
Copy link
Copy Markdown
Collaborator

@Sjors Sjors Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you've lost part of this entire test now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've restored the lost bit

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They still seem missing as of 05585cd.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good catch, let me reset and redo. Thank you

@Sjors
Copy link
Copy Markdown
Collaborator

Sjors commented Feb 16, 2026

It's (slightly) easier to review the deduplication commit if you do it first.

@xyephy xyephy force-pushed the 2025/11/fee-timer-test branch from ef1a258 to cfe50cd Compare February 17, 2026 09:17
// -sv2interval=N requires that we don't send fee updates until at least
// N seconds have gone by. So we first call waitNext() without a fee
// threshold, and then on the next while iteration we set it.
// TODO: add test coverage
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

05585cd: nit, this in the wrong commit now (don't worry about it)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me do a better cleanup, thank you.

xyephy added 2 commits March 31, 2026 17:28
Extract SendSetupConnection(), SendCoinbaseOutputConstraints(), and
ReceiveTemplatePair() into TPTester to reduce duplication in client_tests.
Add TPTester(Sv2TemplateProviderOptions) constructor for custom options.
Remove duplicate tester.handshake() call.
Exercise the -sv2interval timer logic with is_test=false and a 2s
fee_check_interval. Verifies that fee increases are blocked while the
timer is active and allowed after it fires.
@xyephy xyephy force-pushed the 2025/11/fee-timer-test branch from cfe50cd to 1d3013e Compare March 31, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test coverage for fee-based rate limiting timer

2 participants